Repository navigation
Conversation
asgerf
force-pushed
the
asgerf-legacy-mad-property-models
branch
3 times, most recently
from
October 30, 2026 11:02
58c2b3f to
6965a5c
Compare
Property models (e.g. UITextField.text, Process.arguments) are matched against member accesses using type inference on the base. Selector now covers both call-site and property selectors. Also add stubs for all types mentioned in models to the builtins, since dependency declarations are not extracted outside of tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Previously type inference assumed that UnqualifiedMemberAccess could only be qualified by a receiver parameter, which bypasses the abstraction set up in prior phases.
asgerf
force-pushed
the
asgerf-legacy-mad-property-models
branch
from
November 2, 2026 08:23
6965a5c to
a89c40a
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical findings show that length models can generate false injection alerts and builtin names can make references resolve to declarations across otherwise isolated source scopes.
4 open issues
What changed in this PR
Extends Unified’s legacy Swift models to recognize property sources and sinks using inferred receiver types, improving analysis of APIs such as UITextField.text and Process properties.
Changes:
- Adds model selection for property reads, writes, and in-place updates.
- Adds builtin type representatives and implicit-receiver type inference.
- Expands model and command-injection regression coverage with updated expectations.
| File | Description |
|---|---|
unified/tools/builtins/types.swift |
Adds type representatives for legacy Swift models. |
unified/ql/test/query-tests/security/CWE-078/CommandInjection/CommandInjection.swift |
Marks newly detected property sinks as expected alerts. |
unified/ql/test/query-tests/security/CWE-078/CommandInjection/CommandInjection.expected |
Records additional alerts and flow paths. |
unified/ql/test/library-tests/mad/test.swift |
Adds property, subtype, and implicit-self cases. |
unified/ql/test/library-tests/mad/test.ql |
Checks model types for missing builtin representatives. |
unified/ql/test/library-tests/mad/test.expected |
Updates model-selection expectations. |
unified/ql/test/library-tests/mad-builtins/test.swift |
Tests property models without dependency stubs. |
unified/ql/test/library-tests/mad-builtins/test.qlref |
Reuses the model query and expectation checker. |
unified/ql/test/library-tests/mad-builtins/test.expected |
Records builtin-backed source and sink results. |
unified/ql/src/queries/security/CWE-078/CommandInjection.ql |
Removes the obsolete property-sink TODO; query behavior is unchanged. |
unified/ql/lib/codeql/unified/internal/typeinference/TypeInference.qll |
Exposes variable and member-base type inference. |
unified/ql/lib/codeql/unified/internal/mad/LegacyMaD.qll |
Adds type-aware property selectors and source/sink extraction. |
Review effort: Max (auto)
| struct Root {} | ||
| } | ||
|
|
||
| struct Logger {} |
| * type `cls`. | ||
| */ | ||
| pragma[nomagic] | ||
| private predicate propertyAccessSelector(MemberAccess access, ClassLikeDeclaration cls, string name) { |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds support for sources and sinks in legacy Swift models that refer to properties rather than calls (e.g.
UITextField.text,Process.arguments).LegacyMaD.qllpreviously only understood method/constructor models.We can't ignore the receiver type like we do for method calls, as the imprecision would be too much, even though we're mainly focused on recall at this stage. We thus rely on type inference for properties. To get slightly better type info, we also include built-in representatives for each type that is currently used in a MaD model. This means, for example, class names like
URLcan be resolved if it appears as the return type of a function within the repo, but calls to external functions that returnURLare still not known to returnURL.